Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make sure helpers return strict string #438

Merged
merged 1 commit into from
Apr 29, 2015
Merged

make sure helpers return strict string #438

merged 1 commit into from
Apr 29, 2015

Conversation

dead-horse
Copy link
Member

not sure if this is look good to everyone, but it reduce lots of code like:

(this.charset || '').toLowerCase();
(this.get('ua') || '').indexOf('chrome');
(this.type || '').match(/^text/);

and this won't break your app if your are using !this.get('referer') etc.

@jonathanong
Copy link
Member

LGTM. can't see any possible breaking changes. anyone else?

@targos
Copy link
Contributor

targos commented Apr 28, 2015

LGTM but I think there are missing return ''; in request.get(field)

@juliangruber
Copy link
Contributor

LGTM

@juliangruber
Copy link
Contributor

Strictly it's still a breaking change because we didn't tell people to use lose equality checks, if someone is strictly comparing typeof this.get('referer') === 'undefined' it breaks. i'm not sure though if someone does that even.

@fengmk2
Copy link
Member

fengmk2 commented Apr 29, 2015

LGTM

@tj
Copy link
Member

tj commented Apr 29, 2015

👍

dead-horse added a commit that referenced this pull request Apr 29, 2015
@dead-horse
Merge pull request #438 from koajs/strict
e21fc8a 
make sure helpers return strict string
@dead-horse dead-horse merged commit e21fc8a into master Apr 29, 2015
@dead-horse dead-horse deleted the strict branch April 29, 2015 07:25
@dead-horse
Copy link
Member Author

0.20.0 ?

@jonathanong
Copy link
Member

go for it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dead-horse @jonathanong @targos @juliangruber @fengmk2 @tj